Fix, simplify and optimize AbstractRegionOverlapAssociation #1956
Conversation
Unfortunately people who run minecraft servers aren't known for having much sense. I've seen a lot of people setting the group flag on passthrough and I'm unsure what implications this may have. |
|
I think, if this issue should be fixed, there isn't really a way around removing the region group flag for the passthrough flag. At least with the logic that is introduced with this pr (membership depends on passthrough), I'd say that region groups just don't make sense anymore here (neither was there really a use case in the past). However, you are right that there could currently be (probably dumb and overly complex) region setups that will lose protection when a build including the changes of this pr is used. I also agree that this is unacceptable (especially considering that you've "seen a lot of people setting the group flag on passthrough"). The only solution I see to avoid this is to be smarter in the way handling the case where the region group flag has been set. As a general rule for parsing state flags without a region group flag, the following logic could work:
I think with this logic we can avoid that protection is lost. It could lead to stronger protection in case someone has set the region group flag, which must then be fixed manually if not wanted. I think that would be acceptable behavior. |
|
I've implemented the logic described in my comment above. This is ready for review. |
|
in the months since this was opened i've seen 2 more people using the group flag on passthrough. as weird as it really is, it does currently work as intended and is "guaranteed behavior" by the general documentation on how flags work. while the most recent commit makes the transition "safer", i still think this is a breaking change that probably deserves to go into WG8 instead of a 7.x release. re-reading the original issue that spurred this (and the current docs on non-players in global), couldn't the normal regions and the global region just share a nonplayer protection domain and then the pistons would work again (inside the "normal region"), or am i misunderstanding something? |
Ofc this must be documented, for example here in the blue tip box and maybe here too. WG8 is fine for me.
Yes, that's what I told them as a workaround, see https://discord.com/channels/80853743897149440/470410381597343753/869586730817298503. |
This pr fixes two issues:
Integer.MIN_VALUEinstead of0.In general this can also happen with normal regions when the config setting
use-max-priority-associationis set totrue(e.g. by overlapping a protected region and a higher priority region with passthrough set to allow). As noted in the docs, the global region always behaves as ifuse-max-priority-associationis set totrue(due to legacy reasons I guess). That's why this also happens for the global region, even ifuse-max-priority-associationis set tofalse. My solution to this problem is to ignore regions with passthrough set to allow, when calculating the maximum priority here and later checking for>=here. Equivalently the global region is fixed by treating nonplayers as members not only when the source of regions is empty, but also when the source only contains regions with passthrough set to allow (which I calleffectivelyEmpty).I was also able to simplify and optimize some parts while fixing those issues and I have cleaned up a bit. E.g.
#getAssociation(List<ProtectedRegion>)now uses the same logic, regardless of whether or notuse-max-priority-associationis enabled (it's just controlled by the calculated maximum priority). Also calculating the maximum priority returns fast ifuse-max-priority-associationis disabled and it no longer allocates a newHashSet.Unfortunately, checking flags (like passthrough) can itself depend on membership (
#getAssociation), if there's an associated region group flag. This would lead to endless recursions when doing this while calculating the membership. That's why I have removed the passthrough region group flag, which explains the required changes in Flags and SimpleFlagRegistry in case someone has set it. However, I think setting a region group for the passthrough flag doesn't make much sense anyway.